Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CinderSpecCore struct #336

Merged
merged 1 commit into from
Mar 7, 2024

Conversation

dprince
Copy link
Contributor

@dprince dprince commented Feb 13, 2024

This version of the struct (called "core") is meant to be used via the OpenStackControlplane. It is the same as CinderSpec only it is missing the containerImages parameters from the nested API, Backup, Scheduler, and Volume specs.

This is useful to prevent users from interacting with these
parameters directly via the OpenStackControlplane

@dprince
Copy link
Contributor Author

dprince commented Feb 16, 2024

/test cinder-operator-build-deploy-tempest

@dprince dprince force-pushed the core_api branch 3 times, most recently from b701dd4 to f720d56 Compare February 23, 2024 15:31
Copy link
Contributor

@ASBishop ASBishop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dan, I assume this is all part of the migration to using "openstackversions" to manage container images. What's the state of that effort? We don't want to lose the ability to manage cinder-volume's container images if there will be a time lag between merging this PR and when "openstackversions" will be ready.

api/v1beta1/cinderapi_types.go Outdated Show resolved Hide resolved
api/v1beta1/cinderbackup_types.go Outdated Show resolved Hide resolved
api/v1beta1/cinderscheduler_types.go Outdated Show resolved Hide resolved
api/v1beta1/cindervolume_types.go Outdated Show resolved Hide resolved
Copy link
Contributor

@Akrog Akrog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approach looks good to me.
Besides the changes requested by Alan around indentation, I think we should change CinderAPITemplate and other templates to use their Core equivalent to remove code duplication.

api/v1beta1/cinderapi_types.go Outdated Show resolved Hide resolved
api/v1beta1/cinderbackup_types.go Outdated Show resolved Hide resolved
api/v1beta1/cinderscheduler_types.go Outdated Show resolved Hide resolved
api/v1beta1/cindervolume_types.go Outdated Show resolved Hide resolved
This version of the struct (called "core") is meant to
be used via the OpenStackControlplane. It is the same
as CinderSpec only it is missing the containerImages
from the nested API, Backup, Scheduler, and Volume specs.

The Default() function for webhooks has been split accordingly.

Jira: OSPRH-4835
Copy link
Contributor

@Akrog Akrog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Mar 7, 2024
Copy link
Contributor

openshift-ci bot commented Mar 7, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Akrog, dprince

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit 64cafe6 into openstack-k8s-operators:main Mar 7, 2024
7 checks passed
ASBishop pushed a commit to ASBishop/cinder-operator that referenced this pull request Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants